Skip to content

fix-new-chat#2103

Merged
Dal-Papa merged 3 commits into
Expensify:masterfrom
johndev0711:new-chat-2
Mar 31, 2021
Merged

fix-new-chat#2103
Dal-Papa merged 3 commits into
Expensify:masterfrom
johndev0711:new-chat-2

Conversation

@johndev0711

@johndev0711 johndev0711 commented Mar 26, 2021

Copy link
Copy Markdown
Contributor

<If necessary, assign reviewers that know the area or changes well. Feel free to tag any additional reviewers you see fit.>

Details

There are 4 requirements about this GitHub issue, that is, 1. "Remove the feedback... ", 2. "When typing... ... ... show what they've typed below, but greyed out and unselectable.", 3. "... ... ... show the option as selectable(as we do today)", 4. "Phone number validation... 'please enter... ... ... the country code...'"

All of them are related with the getHeaderMessage function in OptionsListUtils.js.

This getHeaderMessage function has 2 props as the original props: hasSelectableOptions, hasUserToInvite.
We have to add a props - searchValue to them, this used to implement the second and fourth requirements.
Also, we have to change the createNewChat function of the NewChatPage.js by adding this.state.searchValue. Here this.state.searchValue has the inputted value. And this value show the feedback according to the second requirement.

For solving the fourth requirement, we have to do validation with the inputted value (searchValue).
This problem has been solved easily by using simple JavaScript validation code.

Additionally, I've changed the default messages in CONST.js with the message of the fourth requirement.

Fixed Issues

#1968

Tests

Running the web app: npm run build/npm run web
Running the MacOS desktop app: npm run desktop-build/npm run desktop
Running the iOS app: npm run ios-build/npm run ios
Android app: npm run android-build /npm run android

  1. Run and click the (+) /New Chat button.
  2. Click on the text input to bring it into focus
  3. Type the text which exist on the list, then the below list would be filtered. (Ex: "toti")
  4. When typing, if the input does not match a valid email or phone number format then show what they've typed below, but greyed out and unselectable. Once the input matches a valid email or phone number format then show the option as selectable (as we do today)
    For example, abcde@fgs : show what they've typed below, but greyed out and unseletable.
    abcde@fgs.com : show the option as selectable.
  5. If the input is only numbers and begins with 0, then display message Please enter a phone number including the country code e.g +447814266907
    For example, 123..(.number) => Show the selectable phone number, 012...(number) => Show message please enter ...,

All testing results have added as following attachments(screenshots)

Tested On

  • Web
  • Mobile Web
  • Desktop
  • iOS
  • Android

Screenshots

Web

image
image
image
image
image
image
image
image
image

Mobile Web

image
image
image
image
image
image
image
image

Desktop

image
image
image
image
image
image

iOS

image

image
image

image

image

image

image

Android

image
image
image
image
image
image
image

@johndev0711 johndev0711 requested a review from a team as a code owner March 26, 2021 09:22
@botify botify requested review from Dal-Papa and removed request for a team March 26, 2021 09:22
@johndev0711

Copy link
Copy Markdown
Contributor Author

I have read the CLA Document and I hereby sign the CLA

@johndev0711 johndev0711 reopened this Mar 26, 2021
@puneetlath

Copy link
Copy Markdown
Contributor

Hey @johndev0711 can you please fill in the PR template? You need to link the GH issue that your PR is for as well as provide instructions on how to test your PR and screenshots from each of the platforms listed to show that it works on each platform. Thanks!

@johndev0711

johndev0711 commented Mar 28, 2021

Copy link
Copy Markdown
Contributor Author

I've filled in the PR template already. Thanks

@johndev0711 johndev0711 reopened this Mar 28, 2021
@MariaHCD MariaHCD mentioned this pull request Mar 29, 2021
5 tasks

if (!hasSelectableOptions && !hasUserToInvite) {
return CONST.MESSAGES.NO_CONTACTS_FOUND;
if (/^\d+$/.test(searchValue)) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this check if it starts with a 0 ? I don't see it here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, you're right.
If it starts with a '0', the country code validation function runs. The function exist already in source code.
Thanks for your kindness.

* @return {String}
*/
function getHeaderMessage(hasSelectableOptions, hasUserToInvite, maxParticipantsReached = false) {
function getHeaderMessage(hasSelectableOptions, hasUserToInvite, searchValue, maxParticipantsReached = false) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we change a method signature we must update all usages. 🙃

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, yes, I've just checked it.
I've thought the only newChat function.
Your concern can be solved easily by adding a props(this.state.searchValue) in the NewGroupPage.js.
image
Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants